Skip to content

CI for unit tests#28

Merged
Szelethus merged 6 commits intoEricsson:mainfrom
furtib:ci_unit_test
Aug 7, 2025
Merged

CI for unit tests#28
Szelethus merged 6 commits intoEricsson:mainfrom
furtib:ci_unit_test

Conversation

@furtib
Copy link
Copy Markdown
Contributor

@furtib furtib commented Jul 28, 2025

We should run the unit tests for every commit!

I have added the update-alternative command, because by default, clang-extdef-mapping was only accessible through clang-extdef-mapping-18

removed --stat option, opensource clang doesn't support this flag

While all tests are passing, keep #26 in mind! It only passes because we don't install cppcheck or Infer.

@Szelethus Szelethus requested review from Szelethus and nettle July 28, 2025 14:03
Copy link
Copy Markdown
Contributor

@Szelethus Szelethus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whats the gain of blanket copy-pasting the entire Analysis job including printing the versions? I'm fine not creating a new job, and simply adding the test runs on the end.

While we are still hashing out the test directory structure in #22, I suppose I'm more in favour of finally running these tests on every PR before we rework everything. But, this is also no hill I intend to die on.

Comment thread .github/workflows/test.yml Outdated
Comment thread .github/workflows/test.yml Outdated
Comment thread .github/workflows/test.yml Outdated
Comment thread .github/workflows/test.yml Outdated
Comment thread test/BUILD
Copy link
Copy Markdown
Contributor

@Szelethus Szelethus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that the patch is leaner.

Comment thread .github/workflows/test.yml Outdated
Comment thread test/BUILD
Comment thread .github/workflows/test.yml Outdated
Comment thread .github/workflows/test.yml Outdated
@furtib furtib requested a review from Szelethus July 29, 2025 12:22
@furtib furtib self-assigned this Jul 29, 2025
Copy link
Copy Markdown
Contributor

@Szelethus Szelethus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concerns have been answered/fixed adaquetely. @nettle, what would you like to see before landing this?

Comment thread README.md Outdated
@nettle nettle changed the title Ci for unit tests CI for unit tests Jul 29, 2025
nettle
nettle previously requested changes Jul 29, 2025
Comment thread test/BUILD
Comment thread README.md Outdated
Comment thread .github/workflows/test.yml
@nettle
Copy link
Copy Markdown
Collaborator

nettle commented Jul 30, 2025

Please review #37

This was referenced Jul 30, 2025
@Szelethus Szelethus requested a review from nettle August 5, 2025 09:14
@Szelethus
Copy link
Copy Markdown
Contributor

@nettle, I commented with my recommendations on where to make adjustments to the documentation. It seems to me that nothing that test.yml does is a consequence of the patch, but rather the consequence of the platform. I think we should revert the documentation change to README.md (because I'd prefer to land #13 which does it anyway), and land this.

What do you think?

@nettle nettle dismissed their stale review August 6, 2025 14:16

Please do not treat this as "Request to change"

Copy link
Copy Markdown
Contributor

@Szelethus Szelethus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the README.md change. Otherwise LGTM

Comment thread README.md Outdated
@furtib
Copy link
Copy Markdown
Contributor Author

furtib commented Aug 7, 2025

I removed the line and also rebased from main

@Szelethus Szelethus merged commit bb9a6af into Ericsson:main Aug 7, 2025
1 check passed
furtib added a commit to furtib/rules_codechecker that referenced this pull request Aug 7, 2025
furtib added a commit to furtib/rules_codechecker that referenced this pull request Aug 12, 2025
furtib added a commit to furtib/rules_codechecker that referenced this pull request Aug 27, 2025
We should run the unit tests for every commit!

I have added the update-alternative command, because by default,
`clang-extdef-mapping` was only accessible through
`clang-extdef-mapping-18`

removed `--stat` option, opensource clang doesn't support this flag

While all tests are passing, keep Ericsson#26 in mind! It only passes because we
don't install cppcheck or Infer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants